From 5b0a11ef1475af69b7ca84f1535a99c1d4469b83 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Fri, 28 Jul 2006 12:54:58 +0100 Subject: [PATCH] [PCI] Allow per-device configuration for fine-grained control over PCI configuration space writes, with a goal that was also previously described by Ryan: "Permissive mode should only be used as a fall back for unknown devices. I think the correct solution for dealing with these device-specific configuration space registers is to identify them and add the device-specific fields to the overlay. This patch adds a special configuration space handler for network cards based on the tg3 linux network device driver. This handler should allow for reads/writes to all of the configuration space registers that the tg3 driver requires." This patch attempts to address concerns with Ryan's original submission by moving policy from the dom0 kernel into dom0 user-space. As new quirky devices emerge they can be incorporated into the user-space policy. An added benefit is that changes to the policy are effective for domains created after the changes are written (no need rebuild the hypervisor or restart xend). Signed-off-by: Chris Bookholt --- .../drivers/xen/pciback/Makefile | 3 +- .../drivers/xen/pciback/conf_space.c | 69 +++++--- .../drivers/xen/pciback/conf_space.h | 11 +- .../drivers/xen/pciback/conf_space_quirks.c | 128 ++++++++++++++ .../drivers/xen/pciback/conf_space_quirks.h | 35 ++++ .../drivers/xen/pciback/pci_stub.c | 156 +++++++++++++++++- .../drivers/xen/pciback/pciback.h | 1 + 7 files changed, 375 insertions(+), 28 deletions(-) create mode 100644 linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.c create mode 100644 linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.h diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/Makefile b/linux-2.6-xen-sparse/drivers/xen/pciback/Makefile index 42f4756707..b1d009d4ef 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pciback/Makefile +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/Makefile @@ -4,7 +4,8 @@ pciback-y := pci_stub.o pciback_ops.o xenbus.o pciback-y += conf_space.o conf_space_header.o \ conf_space_capability.o \ conf_space_capability_vpd.o \ - conf_space_capability_pm.o + conf_space_capability_pm.o \ + conf_space_quirks.o pciback-$(CONFIG_XEN_PCIDEV_BACKEND_VPCI) += vpci.o pciback-$(CONFIG_XEN_PCIDEV_BACKEND_PASS) += passthrough.o diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c index fb47c40ff4..c890001f54 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c @@ -13,6 +13,7 @@ #include #include "pciback.h" #include "conf_space.h" +#include "conf_space_quirks.h" static int permissive = 0; module_param(permissive, bool, 0644); @@ -81,7 +82,7 @@ static int conf_space_write(struct pci_dev *dev, case 4: if (field->u.dw.write) ret = field->u.dw.write(dev, offset, value, - entry->data); + entry->data); break; } return ret; @@ -261,38 +262,58 @@ int pciback_config_write(struct pci_dev *dev, int offset, int size, u32 value) switch (size) { case 1: err = pci_write_config_byte(dev, offset, - (u8)value); + (u8) value); break; case 2: err = pci_write_config_word(dev, offset, - (u16)value); + (u16) value); break; case 4: err = pci_write_config_dword(dev, offset, - (u32)value); + (u32) value); break; } } else if (!dev_data->warned_on_write) { dev_data->warned_on_write = 1; - dev_warn(&dev->dev, "Driver wrote to a read-only " - "configuration space field!\n"); - dev_warn(&dev->dev, "Write at offset 0x%x size %d\n", - offset, size); - dev_warn(&dev->dev, "This may be harmless, but if\n"); - dev_warn(&dev->dev, "you have problems with your " - "device:\n"); - dev_warn(&dev->dev, "1) see the permissive " - "attribute in sysfs.\n"); - dev_warn(&dev->dev, "2) report problems to the " - "xen-devel mailing list along\n"); - dev_warn(&dev->dev, " with details of your device " - "obtained from lspci.\n"); + dev_warn(&dev->dev, "Driver tried to write to a " + "read-only configuration space field at offset " + "0x%x, size %d. This may be harmless, but if " + "you have problems with your device:\n" + "1) see permissive attribute in sysfs\n" + "2) report problems to the xen-devel " + "mailing list along with details of your " + "device obtained from lspci.\n", offset, size); } } return pcibios_err_to_errno(err); } +void pciback_config_free_dyn_fields(struct pci_dev *dev) +{ + struct pciback_dev_data *dev_data = pci_get_drvdata(dev); + struct config_field_entry *cfg_entry, *t; + struct config_field *field; + + dev_dbg(&dev->dev, + "free-ing dynamically allocated virtual configuration space fields\n"); + + list_for_each_entry_safe(cfg_entry, t, &dev_data->config_fields, list) { + field = cfg_entry->field; + + if (field->clean) { + field->clean(field); + + if (cfg_entry->data) + kfree(cfg_entry->data); + + list_del(&cfg_entry->list); + kfree(cfg_entry); + } + + } +} + void pciback_config_reset_dev(struct pci_dev *dev) { struct pciback_dev_data *dev_data = pci_get_drvdata(dev); @@ -338,6 +359,10 @@ int pciback_config_add_field_offset(struct pci_dev *dev, struct config_field_entry *cfg_entry; void *tmp; + /* silently ignore duplicate fields */ + if (pciback_field_is_dup(dev, field->offset)) + goto out; + cfg_entry = kmalloc(sizeof(*cfg_entry), GFP_KERNEL); if (!cfg_entry) { err = -ENOMEM; @@ -388,6 +413,10 @@ int pciback_config_init_dev(struct pci_dev *dev) goto out; err = pciback_config_capability_add_fields(dev); + if (err) + goto out; + + err = pciback_config_quirks_init(dev); out: return err; @@ -395,9 +424,5 @@ int pciback_config_init_dev(struct pci_dev *dev) int pciback_config_init(void) { - int err; - - err = pciback_config_capability_init(); - - return err; + return pciback_config_capability_init(); } diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h index 5279e0edfc..76fe5d810a 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h @@ -33,11 +33,13 @@ typedef int (*conf_byte_read) (struct pci_dev * dev, int offset, u8 * value, * values. */ struct config_field { - unsigned int offset; - unsigned int size; - conf_field_init init; + unsigned int offset; + unsigned int size; + unsigned int mask; + conf_field_init init; conf_field_reset reset; - conf_field_free release; + conf_field_free release; + void (*clean) (struct config_field * field); union { struct { conf_dword_write write; @@ -52,6 +54,7 @@ struct config_field { conf_byte_read read; } b; } u; + struct list_head list; }; struct config_field_entry { diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.c b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.c new file mode 100644 index 0000000000..4ec0c78b89 --- /dev/null +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.c @@ -0,0 +1,128 @@ +/* + * PCI Backend - Handle special overlays for broken devices. + * + * Author: Ryan Wilson + * Author: Chris Bookholt + */ + +#include +#include +#include "pciback.h" +#include "conf_space.h" +#include "conf_space_quirks.h" + +LIST_HEAD(pciback_quirks); + +struct pciback_config_quirk *pciback_find_quirk(struct pci_dev *dev) +{ + struct pciback_config_quirk *tmp_quirk; + + list_for_each_entry(tmp_quirk, &pciback_quirks, quirks_list) + if (pci_match_id(&tmp_quirk->devid, dev)) + goto out; + tmp_quirk = NULL; + printk(KERN_DEBUG + "quirk didn't match any device pciback knows about\n"); + out: + return tmp_quirk; +} + +static inline void register_quirk(struct pciback_config_quirk *quirk) +{ + list_add_tail(&quirk->quirks_list, &pciback_quirks); +} + +int pciback_field_is_dup(struct pci_dev *dev, int reg) +{ + int ret = 0; + struct pciback_dev_data *dev_data = pci_get_drvdata(dev); + struct config_field *field; + struct config_field_entry *cfg_entry; + + list_for_each_entry(cfg_entry, &dev_data->config_fields, list) { + field = cfg_entry->field; + if (field->offset == reg) { + ret = 1; + break; + } + } + return ret; +} + +int pciback_config_quirks_add_field(struct pci_dev *dev, struct config_field + *field) +{ + int err = 0; + + switch (field->size) { + case 1: + field->u.b.read = pciback_read_config_byte; + field->u.b.write = pciback_write_config_byte; + break; + case 2: + field->u.w.read = pciback_read_config_word; + field->u.w.write = pciback_write_config_word; + break; + case 4: + field->u.dw.read = pciback_read_config_dword; + field->u.dw.write = pciback_write_config_dword; + break; + default: + err = -EINVAL; + goto out; + } + + pciback_config_add_field(dev, field); + + out: + return err; +} + +int pciback_config_quirks_init(struct pci_dev *dev) +{ + struct pciback_config_quirk *quirk; + int ret = 0; + + quirk = kzalloc(sizeof(*quirk), GFP_ATOMIC); + if (!quirk) { + ret = -ENOMEM; + goto out; + } + + quirk->devid.vendor = dev->vendor; + quirk->devid.device = dev->device; + quirk->devid.subvendor = dev->subsystem_vendor; + quirk->devid.subdevice = dev->subsystem_device; + quirk->devid.class = 0; + quirk->devid.class_mask = 0; + quirk->devid.driver_data = 0UL; + + quirk->pdev = dev; + + register_quirk(quirk); + out: + return ret; +} + +void pciback_config_field_free(struct config_field *field) +{ + kfree(field); +} + +int pciback_config_quirk_release(struct pci_dev *dev) +{ + struct pciback_config_quirk *quirk; + int ret = 0; + + quirk = pciback_find_quirk(dev); + if (!quirk) { + ret = -ENXIO; + goto out; + } + + list_del(&quirk->quirks_list); + kfree(quirk); + + out: + return ret; +} diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.h b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.h new file mode 100644 index 0000000000..5c47e1269b --- /dev/null +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.h @@ -0,0 +1,35 @@ +/* + * PCI Backend - Data structures for special overlays for broken devices. + * + * Ryan Wilson + * Chris Bookholt + */ + +#ifndef __XEN_PCIBACK_CONF_SPACE_QUIRKS_H__ +#define __XEN_PCIBACK_CONF_SPACE_QUIRKS_H__ + +#include +#include + +struct pciback_config_quirk { + struct list_head quirks_list; + struct pci_device_id devid; + struct pci_dev *pdev; +}; + +struct pciback_config_quirk *pciback_find_quirk(struct pci_dev *dev); + +int pciback_config_quirks_add_field(struct pci_dev *dev, struct config_field + *field); + +int pciback_config_quirks_remove_field(struct pci_dev *dev, int reg); + +int pciback_config_quirks_init(struct pci_dev *dev); + +void pciback_config_field_free(struct config_field *field); + +int pciback_config_quirk_release(struct pci_dev *dev); + +int pciback_field_is_dup(struct pci_dev *dev, int reg); + +#endif diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c b/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c index 0f92f82dd2..ff6db118c0 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c @@ -1,7 +1,8 @@ /* * PCI Stub Driver - Grabs devices in backend to be exported later * - * Author: Ryan Wilson + * Ryan Wilson + * Chris Bookholt */ #include #include @@ -10,6 +11,8 @@ #include #include #include "pciback.h" +#include "conf_space.h" +#include "conf_space_quirks.h" static char *pci_devs_to_hide = NULL; module_param_named(hide, pci_devs_to_hide, charp, 0444); @@ -31,6 +34,7 @@ struct pcistub_device { struct pci_dev *dev; struct pciback_device *pdev; /* non-NULL if struct pci_dev is in use */ }; + /* Access to pcistub_devices & seized_devices lists and the initialize_devices * flag must be locked with pcistub_devices_lock */ @@ -76,6 +80,7 @@ static void pcistub_device_release(struct kref *kref) /* Clean-up the device */ pciback_reset_device(psdev->dev); + pciback_config_free_dyn_fields(psdev->dev); pciback_config_free_dev(psdev->dev); kfree(pci_get_drvdata(psdev->dev)); pci_set_drvdata(psdev->dev, NULL); @@ -95,6 +100,32 @@ static inline void pcistub_device_put(struct pcistub_device *psdev) kref_put(&psdev->kref, pcistub_device_release); } +static struct pcistub_device *pcistub_device_find(int domain, int bus, + int slot, int func) +{ + struct pcistub_device *psdev = NULL; + unsigned long flags; + + spin_lock_irqsave(&pcistub_devices_lock, flags); + + list_for_each_entry(psdev, &pcistub_devices, dev_list) { + if (psdev->dev != NULL + && domain == pci_domain_nr(psdev->dev->bus) + && bus == psdev->dev->bus->number + && PCI_DEVFN(slot, func) == psdev->dev->devfn) { + pcistub_device_get(psdev); + goto out; + } + } + + /* didn't find it */ + psdev = NULL; + + out: + spin_unlock_irqrestore(&pcistub_devices_lock, flags); + return psdev; +} + static struct pci_dev *pcistub_device_get_pci_dev(struct pciback_device *pdev, struct pcistub_device *psdev) { @@ -180,6 +211,7 @@ void pcistub_put_pci_dev(struct pci_dev *dev) * (so it's ready for the next domain) */ pciback_reset_device(found_psdev->dev); + pciback_config_free_dyn_fields(found_psdev->dev); pciback_config_reset_dev(found_psdev->dev); spin_lock_irqsave(&found_psdev->lock, flags); @@ -392,6 +424,8 @@ static void pcistub_remove(struct pci_dev *dev) spin_lock_irqsave(&pcistub_devices_lock, flags); + pciback_config_quirk_release(dev); + list_for_each_entry(psdev, &pcistub_devices, dev_list) { if (psdev->dev == dev) { found_psdev = psdev; @@ -471,6 +505,19 @@ static inline int str_to_slot(const char *buf, int *domain, int *bus, return -EINVAL; } +static inline int str_to_quirk(const char *buf, int *domain, int *bus, int + *slot, int *func, int *reg, int *size, int *mask) +{ + int err; + + err = + sscanf(buf, " %04x:%02x:%02x.%1x-%08x:%1x:%08x", domain, bus, slot, + func, reg, size, mask); + if (err == 7) + return 0; + return -EINVAL; +} + static int pcistub_device_id_add(int domain, int bus, int slot, int func) { struct pcistub_device_id *pci_dev_id; @@ -523,6 +570,46 @@ static int pcistub_device_id_remove(int domain, int bus, int slot, int func) return err; } +static int pcistub_reg_add(int domain, int bus, int slot, int func, int reg, + int size, int mask) +{ + int err = 0; + struct pcistub_device *psdev; + struct pci_dev *dev; + struct config_field *field; + + psdev = pcistub_device_find(domain, bus, slot, func); + if (!psdev || !psdev->dev) { + err = -ENODEV; + goto out; + } + dev = psdev->dev; + + /* check for duplicate field */ + if (pciback_field_is_dup(dev, reg)) + goto out; + + field = kzalloc(sizeof(*field), GFP_ATOMIC); + if (!field) { + err = -ENOMEM; + goto out; + } + + field->offset = reg; + field->size = size; + field->mask = mask; + field->init = NULL; + field->reset = NULL; + field->release = NULL; + field->clean = pciback_config_field_free; + + err = pciback_config_quirks_add_field(dev, field); + if (err) + kfree(field); + out: + return err; +} + static ssize_t pcistub_slot_add(struct device_driver *drv, const char *buf, size_t count) { @@ -587,6 +674,71 @@ static ssize_t pcistub_slot_show(struct device_driver *drv, char *buf) DRIVER_ATTR(slots, S_IRUSR, pcistub_slot_show, NULL); +static ssize_t pcistub_quirk_add(struct device_driver *drv, const char *buf, + size_t count) +{ + int domain, bus, slot, func, reg, size, mask; + int err; + + err = str_to_quirk(buf, &domain, &bus, &slot, &func, ®, &size, + &mask); + if (err) + goto out; + + err = pcistub_reg_add(domain, bus, slot, func, reg, size, mask); + + out: + if (!err) + err = count; + return err; +} + +static ssize_t pcistub_quirk_show(struct device_driver *drv, char *buf) +{ + int count = 0; + unsigned long flags; + extern struct list_head pciback_quirks; + struct pciback_config_quirk *quirk; + struct pciback_dev_data *dev_data; + struct config_field *field; + struct config_field_entry *cfg_entry; + + spin_lock_irqsave(&device_ids_lock, flags); + list_for_each_entry(quirk, &pciback_quirks, quirks_list) { + if (count >= PAGE_SIZE) + goto out; + + count += scnprintf(buf + count, PAGE_SIZE - count, + "%02x:%02x.%01x\n\t%04x:%04x:%04x:%04x\n", + quirk->pdev->bus->number, + PCI_SLOT(quirk->pdev->devfn), + PCI_FUNC(quirk->pdev->devfn), + quirk->devid.vendor, quirk->devid.device, + quirk->devid.subvendor, + quirk->devid.subdevice); + + dev_data = pci_get_drvdata(quirk->pdev); + + list_for_each_entry(cfg_entry, &dev_data->config_fields, list) { + field = cfg_entry->field; + if (count >= PAGE_SIZE) + goto out; + + count += scnprintf(buf + count, PAGE_SIZE - + count, "\t\t%08x:%01x:%08x\n", + field->offset, field->size, + field->mask); + } + } + + out: + spin_unlock_irqrestore(&device_ids_lock, flags); + + return count; +} + +DRIVER_ATTR(quirks, S_IRUSR | S_IWUSR, pcistub_quirk_show, pcistub_quirk_add); + static int __init pcistub_init(void) { int pos = 0; @@ -631,6 +783,7 @@ static int __init pcistub_init(void) driver_create_file(&pciback_pci_driver.driver, &driver_attr_remove_slot); driver_create_file(&pciback_pci_driver.driver, &driver_attr_slots); + driver_create_file(&pciback_pci_driver.driver, &driver_attr_quirks); out: return err; @@ -680,6 +833,7 @@ static void __exit pciback_cleanup(void) driver_remove_file(&pciback_pci_driver.driver, &driver_attr_remove_slot); driver_remove_file(&pciback_pci_driver.driver, &driver_attr_slots); + driver_remove_file(&pciback_pci_driver.driver, &driver_attr_quirks); pci_unregister_driver(&pciback_pci_driver); } diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h index 3fb1e903eb..18c0715b46 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h @@ -61,6 +61,7 @@ void pciback_reset_device(struct pci_dev *pdev); /* Access a virtual configuration space for a PCI device */ int pciback_config_init(void); int pciback_config_init_dev(struct pci_dev *dev); +void pciback_config_free_dyn_fields(struct pci_dev *dev); void pciback_config_reset_dev(struct pci_dev *dev); void pciback_config_free_dev(struct pci_dev *dev); int pciback_config_read(struct pci_dev *dev, int offset, int size, -- 2.30.2